Conversation
507e5df to
0a768b6
Compare
0a768b6 to
1611819
Compare
4f80f69 to
30d2f1d
Compare
There was a problem hiding this comment.
Pull request overview
Adds Datadog RUM instrumentation to Ledger Live Desktop (renderer + main error forwarding), while extending the shared @ledgerhq/client-ids library to manage/persist UserId and DatadogId and exposing a new lldDatadog feature flag configuration.
Changes:
- Add Desktop Datadog RUM init/tagging + forward main-process errors to renderer for reporting.
- Extend
@ledgerhq/client-idsidentities store to includeuserId/datadogId, persistence, selectors, and init/migration helper on desktop. - Update build/test scaffolding (env injection globals, Jest globals) and add
@datadog/browser-rumdependency.
Reviewed changes
Copilot reviewed 48 out of 51 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Locks @datadog/browser-rum and its transitive deps. |
| libs/ledgerjs/packages/types-live/src/feature.ts | Adds lldDatadog feature flag type definition. |
| libs/ledger-live-common/src/featureFlags/defaultFeatures.ts | Adds default params for lldDatadog. |
| libs/client-ids/src/store/types.ts | Adds userId/datadogId to identities state + dummy placeholders/helpers. |
| libs/client-ids/src/store/slice.ts | Loads/persist/migrates userId/datadogId; adds init actions. |
| libs/client-ids/src/store/selectors.ts | Adds selectors for userId and datadogId. |
| libs/client-ids/src/store/persistence.ts | Persists userId/datadogId (omits dummy values). |
| libs/client-ids/src/store/persistence.test.ts | Tests persistence behavior for new IDs. |
| libs/client-ids/src/store/middleware.ts | Changes sync gating to depend on userId being non-dummy. |
| libs/client-ids/src/store/middleware.test.ts | Updates middleware tests to use initFromScratch vs async getUserId. |
| libs/client-ids/src/store/index.ts | Re-exports selectors. |
| libs/client-ids/src/ids/index.ts | Exports new ID wrappers. |
| libs/client-ids/src/ids/UserId.ts | Introduces UserId wrapper with redacted toString/toJSON. |
| libs/client-ids/src/ids/DatadogId.ts | Introduces DatadogId wrapper + allowlisted export methods. |
| libs/client-ids/src/ids/DatadogId.test.ts | Adds unit tests for DatadogId. |
| libs/client-ids/export-rules.json | Updates allowlist rules for new export methods. |
| apps/ledger-live-desktop/tools/rspack/utils.ts | Loads .env* files and injects Datadog globals in builds. |
| apps/ledger-live-desktop/tools/dist/index.js | Ensures Datadog env vars are passed into production/staging builds. |
| apps/ledger-live-desktop/tests/testSetup.tsx | Exposes unmount in test render return type. |
| apps/ledger-live-desktop/tests/mocks/electron.ts | Adjusts mocked ipcRenderer.invoke typing/behavior. |
| apps/ledger-live-desktop/src/state-manager/configureStore.ts | Stops passing legacy getUserId to identities sync middleware. |
| apps/ledger-live-desktop/src/sentry/renderer.ts | Switches Sentry user id to DatadogId from identities store. |
| apps/ledger-live-desktop/src/sentry/anonymizer.ts | Makes electron IPC path lookup more defensive; adjusts path replace. |
| apps/ledger-live-desktop/src/sentry/anonymizer.test.ts | Adds tests for anonymizer path replacement/recursion. |
| apps/ledger-live-desktop/src/renderer/screens/swapWeb/index.tsx | Reports swap load error to both Sentry and Datadog. |
| apps/ledger-live-desktop/src/renderer/screens/exchange/Swap2/Form/SwapWebViewDemo3.tsx | Reports swap unavailable error to both Sentry and Datadog. |
| apps/ledger-live-desktop/src/renderer/reducers/settings.ts | Notes sentryLogs flag is also used as Datadog opt-in. |
| apps/ledger-live-desktop/src/renderer/logger/index.ts | Sends breadcrumbs/errors to Datadog alongside Sentry. |
| apps/ledger-live-desktop/src/renderer/logger/index.test.ts | Adds unit tests for logger Datadog/Sentry calls. |
| apps/ledger-live-desktop/src/renderer/init.tsx | Introduces identities initialization helper; updates Sentry init call signature. |
| apps/ledger-live-desktop/src/renderer/helpers/identities.ts | Adds desktop identities initialization/migration logic. |
| apps/ledger-live-desktop/src/renderer/helpers/identities.test.ts | Adds tests for identities init/migration paths. |
| apps/ledger-live-desktop/src/renderer/components/ConnectEnvsToDatadog.tsx | Initializes Datadog (opt-in + feature flag) and syncs tags periodically. |
| apps/ledger-live-desktop/src/renderer/components/ConnectEnvsToDatadog.test.tsx | Adds tests for Datadog init + tag syncing + cleanup. |
| apps/ledger-live-desktop/src/renderer/App.tsx | Wires ConnectEnvsToDatadog into app providers tree. |
| apps/ledger-live-desktop/src/main/setup.ts | Captures crash test errors in Datadog path too; minor Node import cleanup. |
| apps/ledger-live-desktop/src/main/index.ts | Initializes Datadog main forwarding based on persisted datadogId. |
| apps/ledger-live-desktop/src/datadog/renderer.ts | Adds Datadog RUM init, IPC error ingestion, breadcrumbs/tags helpers. |
| apps/ledger-live-desktop/src/datadog/renderer.test.ts | Adds tests for Datadog renderer wrapper behavior. |
| apps/ledger-live-desktop/src/datadog/main.ts | Adds main-process error capture/forwarding to renderer. |
| apps/ledger-live-desktop/src/datadog/main.test.ts | Adds tests for Datadog main forwarding and gating. |
| apps/ledger-live-desktop/src/datadog/ignoreErrors.ts | Ports Sentry-like ignore list for Datadog filtering. |
| apps/ledger-live-desktop/src/datadog/ignoreErrors.test.ts | Adds tests for ignore list matcher. |
| apps/ledger-live-desktop/src/datadog/config.ts | Adds Datadog build config accessor + beforeSend anonymization/filtering. |
| apps/ledger-live-desktop/src/datadog/config.test.ts | Adds tests for Datadog config and beforeSend. |
| apps/ledger-live-desktop/package.json | Adds @datadog/browser-rum dependency. |
| apps/ledger-live-desktop/jest.config.js | Adds Datadog globals for tests. |
| apps/ledger-live-desktop/index-types.d.ts | Declares Datadog build-time globals. |
| apps/ledger-live-desktop/.eslintrc.js | Marks Datadog globals as readonly for ESLint. |
| apps/ledger-live-desktop/.env.staging | Adds Datadog RUM env vars for staging. |
| apps/ledger-live-desktop/.env.production | Adds Datadog RUM env vars for production. |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
| jest.mock("~/sentry/renderer", () => ({ | ||
| captureException: jest.fn(), | ||
| captureBreadcrumb: jest.fn(), | ||
| })); |
There was a problem hiding this comment.
~/sentry/renderer is already mocked globally in apps/ledger-live-desktop/tests/jestSetup.js (as src/sentry/renderer). Re-mocking it here can cause mock cannibalization/flakiness with parallel Jest workers. Prefer using the global mock and only configure return values/expectations via jest.mocked(...) in beforeEach.
| jest.mock("~/renderer/experimental", () => ({ | ||
| enabledExperimentalFeatures: jest.fn(() => []), | ||
| })); | ||
|
|
There was a problem hiding this comment.
This test file mocks ~/renderer/experimental twice; the first mock is immediately overridden by the second. Consolidating into a single jest.mock will make the test setup clearer and avoid confusion about which implementation is active.
| jest.mock("~/renderer/experimental", () => ({ | |
| enabledExperimentalFeatures: jest.fn(() => []), | |
| })); |
| "test-deep-links": "ws --spa ledger-live-desktop-deeplinks.html" | ||
| }, | ||
| "dependencies": { | ||
| "@datadog/browser-rum": "6.27.1", |
There was a problem hiding this comment.
This PR introduces a new desktop feature/instrumentation path and adds a new dependency (@datadog/browser-rum). Per repo policy, user-facing behavior and library/API changes should include a Changeset (pnpm changeset). No Datadog-related changeset appears to be present under .changeset/ — please add one.
| "@datadog/browser-rum": "6.27.1", |
| * Optional: resolve userId (e.g. from legacy app user). Used by mobile until it migrates to identities store. | ||
| * Desktop uses identities state only and does not pass this. | ||
| */ | ||
| getUserId?: (state: State) => Promise<string>; | ||
| } |
There was a problem hiding this comment.
SyncMiddlewareConfig still exposes getUserId, but the middleware never calls it anymore and also short-circuits syncing when userId is dummy. This breaks current mobile usage (mobile configureStore passes getUserId and may keep identities userId dummy), so push-devices sync will never run. Either remove getUserId everywhere, or (preferred for backward compatibility) when userId is dummy and getUserId is provided, resolve it and dispatch an action (e.g., importFromLegacy / initFromScratch) before proceeding with sync.
| if (persisted) { | ||
| store.dispatch(identitiesSlice.actions.initFromPersisted(persisted)); | ||
| return false; | ||
| } |
There was a problem hiding this comment.
When persisted app/identities exists (older versions only persisted device IDs), this function returns immediately after initFromPersisted, leaving userId/datadogId as the dummy placeholders. That prevents push-devices sync (dummy userId) and prevents Datadog user identification. After loading persisted identities, consider migrating missing IDs (try legacy app/user / localStorage, otherwise generate via initFromScratch or a dedicated “ensureIds” action) before returning.
There was a problem hiding this comment.
i'm not sure i understand this feedback. if it's imported from existing app.identities, it can't be dummy
30d2f1d to
c8f910f
Compare
db01ae9 to
0b8ee4b
Compare
4fed259 to
c0432a2
Compare
7c12cd7 to
5fca434
Compare
c0d044b to
ca790ec
Compare
|
The latest updates on your projects. Learn more about Vercel for GitHub. 3 Skipped Deployments
|
| <<<<<<< HEAD | ||
| sentry(() => sentryLogsSelector(store.getState())); | ||
| ======= | ||
| // Initialize identities before Sentry so setUser gets the real datadogId (not dummy) when available | ||
| await initIdentities(store); | ||
| sentry(() => sentryLogsSelector(store.getState()), store); | ||
| >>>>>>> c0d044b41a (feat(desktop): add Datadog RUM and build tooling) |
There was a problem hiding this comment.
There are unresolved merge conflict markers (<<<<<<< HEAD, =======, >>>>>>> c0d044b41a) left in this file. This will cause a syntax error at runtime and the app will not start. The conflict between the two versions of the sentry(...) call and initIdentities must be resolved before merging.
| <<<<<<< HEAD | |
| sentry(() => sentryLogsSelector(store.getState())); | |
| ======= | |
| // Initialize identities before Sentry so setUser gets the real datadogId (not dummy) when available | |
| await initIdentities(store); | |
| sentry(() => sentryLogsSelector(store.getState()), store); | |
| >>>>>>> c0d044b41a (feat(desktop): add Datadog RUM and build tooling) | |
| // Initialize identities before Sentry so setUser gets the real datadogId (not dummy) when available | |
| await initIdentities(store); | |
| sentry(() => sentryLogsSelector(store.getState()), store); |
pnpm-lock.yaml
Outdated
| '@grpc/grpc-js@1.7.3': | ||
| dependencies: | ||
| '@grpc/proto-loader': 0.7.15 | ||
| '@types/node': 22.19.13 | ||
| '@types/node': 24.10.13 |
There was a problem hiding this comment.
The lockfile shows @types/node resolved version changing from 22.19.13 to 24.10.13 across dozens of transitive entries. None of the package.json changes in this PR modify @types/node. Per repo guidelines, the lockfile diff should be scoped to what was actually changed. Please regenerate the lockfile from a clean pnpm install to ensure only the intended dependency changes (@datadog/browser-rum, uuid, and test tooling) remain.
| "<!DOCTYPE html", | ||
| "Unexpected ''", | ||
| "Unexpected '<'", | ||
| "Service Unvailable", |
There was a problem hiding this comment.
"Service Unvailable" is misspelled — it should be "Service Unavailable". This typo is also present in the Sentry ignore list (sentry/install.ts:56), so you may want to fix it in both places to correctly catch "503 Service Unavailable" error messages.
| "Service Unvailable", | |
| "Service Unavailable", |
ca790ec to
99cf185
Compare
feat(client-ids): add UserId and DatadogId ID classes Add UserId and DatadogId with redaction, export* methods, and export-rules. DeviceId.exportDeviceIdForPersistence allowlist trimmed to persistence only. feat(client-ids): add userId and datadogId to identities store Extend IdentitiesState with userId/datadogId (dummy placeholders). Slice: initFromPersisted, importFromLegacy, initFromScratch. Persistence and sync middleware use identities state; optional getUserId for mobile. fix: ignore build and generated folders in client-ids script feat(desktop): unified identities migrated from legacy storage test(e2e): add identities migration test chore: extend cursor and github rules to mention client-ids paradigm feat(desktop): use datadogId from identities in Sentry Sentry renderer init takes store and sets user id via datadogIdSelector. fix(desktop): omit userId when outside Redux context (crash screen) ExportLogsButton and RenderError can render without a Redux Provider (e.g. AppError crash screen). Use ReactReduxContext to detect store; when missing, render ExportLogsBtnWithoutStore and omit userAnonymousId from export meta. Conditionally render Hard Reset block in RenderError only when hasStore to avoid store-dependent UI outside Provider.
- Datadog RUM: config, main process (init from db), renderer, ConnectEnvsToDatadog, ignoreErrors. - Main: read identities/datadogId from db and init Datadog when present. - Logger: forward to Datadog captureException when enabled. - Env, jest, eslint, rspack, sourcemaps, testSetup unmount type, pnpm-lock.
99cf185 to
adbd318
Compare
|


✅ Checklist
npx changesetwas attached.📝 Description
client-ids DatadogId and UserId integration ( some integrated from feat(client-ids): encapsulate UserId & DatadogId to implement explicit id usage #13325 )
Datadog RUM (desktop)
lldDatadog+ Sentry logs enabled).lldDatadog:sessionSamplingRate,sessionReplaySampleRate(default 0).DatadogId(UUID v4) stored onuserand passed to RUM.Sourcemaps
tools/distwhenDATADOG_API_KEYis set (service/release-version/minified prefix).DATADOG_API_KEYset in release-desktop and pre-desktop workflows (macOS, Linux, Windows) so CI runs the upload.Sentry unchanged; both run in parallel.
❓ Context
🧐 Checklist for the PR Reviewers